[a11y] Support TalkBack reading by word, character, and paragraph#17626
[a11y] Support TalkBack reading by word, character, and paragraph#17626jonahwilliams merged 13 commits intoflutter:masterfrom
Conversation
| result.setChecked(semanticsNode.hasFlag(Flag.IS_TOGGLED)); | ||
| result.setClassName("android.widget.Switch"); | ||
| result.setContentDescription(semanticsNode.getValueLabelHint()); | ||
| } else if (!semanticsNode.hasFlag(Flag.SCOPES_ROUTE)) { |
There was a problem hiding this comment.
Does this change now make SemanticsNodes that are scoping a route and have a label focusable?
There was a problem hiding this comment.
I'll update the condition so we don't apply it to those nodes, thanks for jogging my memory a bit on the context
|
/cc @xster Do you want to review this one? |
|
Thanks for the fix @jonahwilliams. We should add a test for this. The good news is that writing Java tests is a lot easier now than before. See https://github.com/flutter/engine/blob/master/shell/platform/android/test/README.md#L24. The tests and dependencies all build in ninja and you can use robolectric, mockito and all that good stuff to construction inject etc. Give it a try. Happy to work with you on it if needed. |
|
@xster I updated run_test to use a configured Java so I can point it at Android Studio's JRE. However now I'm getting: But this is 1.8.0_212-release-1586-b4-5784211 , so it should work according to the readme? |
|
Are you on a mac? If you just hard-call a specific java binary but don't set your JAVA_HOME, it's gonna do really weird things. What does your JAVA_HOME and /usr/libexec/java_home -V return? i.e. I think it's going to be a bit more involved to changing the java instance than just calling a specific binary. If documentation is lacking, adding more docs to let people set JAVA_HOME is probably the least scary option. |
| import test.io.flutter.embedding.engine.FlutterShellArgsTest; | ||
| import test.io.flutter.embedding.engine.PluginComponentTest; | ||
| import test.io.flutter.embedding.engine.dart.DartExecutorTest; | ||
| import io.flutter.view.AccessibilityBridgeTest; |
There was a problem hiding this comment.
I assume the formatter will complain about this
There was a problem hiding this comment.
perhaps ... I will format another day :)
testing/run_tests.py
Outdated
| EXPECTED_VERSION = '1.8' | ||
| # `java -version` is output to stderr. https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4380614 | ||
| version_output = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT) | ||
| version_output = subprocess.check_output([java_path, '-version'], stderr=subprocess.STDOUT) |
There was a problem hiding this comment.
if you really want this, you'll probably need an env map too
There was a problem hiding this comment.
Ahh yeah, with JAVA_HOME set everything is working. Thanks!
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java
Show resolved
Hide resolved
|
I rolled back the run_test.py changes since that wasn't going to be sufficient. Test files are now formatted correctly. I added the necessary test code to simulate semantic updates, though I only added tests for the conditions I changed |
Co-Authored-By: xster <xiao@xster.net>
|
Friendly ping @xster |
xster
left a comment
There was a problem hiding this comment.
doh, I had this tab opened twice and didn't reply. Sorry for the wait. Awesome tests!
| } | ||
|
|
||
| /// The encoding for semantics is described in platform_view_android.cc | ||
| class TestSemanticsUpdate { |
There was a problem hiding this comment.
if we want this mapping to be maintained manually, can we add notes to each of accessibilitybridge.updatesemantics, platformviewandroid.updatesemantics and here to remind maintainers?
There was a problem hiding this comment.
Something like : "This logic is also used in the test fixture at path/to/test.java"?
There was a problem hiding this comment.
Sure. Something like "if you change stuff here, don't forget to update the test file as well"
There was a problem hiding this comment.
I added a comment to platform_view_android.cc, since that defines the encoding format. Elsewhere it is only consumed
|
LGTM! |
The logic in talkback does not handle reading from the text portion of an accessibility node when checking if that node could support reading at different granularities. See: https://github.com/google/talkback/blob/9b1d5132b074174d02886faccf731e814d69363c/talkback/src/main/java/GranularityTraversal.java#L207
Fix this by always setting the content description if something is not a text field.
flutter/flutter#13406
Fixes flutter/flutter#13406